-
Notifications
You must be signed in to change notification settings - Fork 363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add data conversion functions #341
Add data conversion functions #341
Conversation
//converting each character to upper case using for loop | ||
for (auto &c : str) | ||
{ | ||
c = std::toupper(c); // using the function toupper() for conversion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than just using std::toupper()
and std::tolower()
, I think it would be better to implement this manually (as done here, though there are lots of ways to do it). Obviously in "real life" you'd just use the std
function, but the point of this repo is (generally) to implement things ourselves (i.e., sorting a list manually rather than just calling std::sort()
). More good info here.
So, please implement these utilities without using the std
functions. Let me know if you have any questions!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay...I will dive right into it !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good start! A few things to comment on.
I'm subscribed to the PR automatically, so no need to tag me in a separate comment saying it's ready for review.
These are actually conversion functions, so replace file names and comments with data_conversion
.
While you've written the unit tests, you need to add them to CMakeLists.txt
in order to run them. Add the following to the bottom of that file:
# ============================================================================
# Utilities
# ============================================================================
# Data conversion
add_executable(data_conversion
test/utils/data_conversion.cpp)
target_link_libraries(data_conversion test_runner)
This will show you that some of your tests aren't actually passing.
Finally, I think that rather than using test cases to break out the tests into base and non-base cases, you should use the test cases for each function, like this:
TEST_CASE("Make upper case", "[string][data-validation][make-upper-case]")
{
// upper case base cases
// upper case other cases
}
TEST_CASE("Make lower case", "[string][data-validation][make-lower-case]")
{
// lower case base cases
// lower case other cases
}
Finally, a base case should test empty strings and strings with one character in them. Everything else can go under the other comment. And make sure to test other edge cases. Like, what happens with numbers (ints, floats)? What happens with special characters? What happens with spaces?
Let me know if you have any questions.
Alright . I will keep that in mind .
|
|
Alright ! Got it |
Okay ! Got it . |
b6bf98c
to
a106f3f
Compare
See in this thread under "Changes requested" where it says "All checks have failed?" At least one of your unit tests is currently failing, so please fix that. Also, remove the old Finally, please break out the unit tests into base cases (with empty string and single character) and other cases, with everything else. These don't need to be entirely other test cases, just a comment separating, like so:
|
Yes...I am figuring out why the tests are failing . |
I am getting this error |
You still have |
This issue has been automatically marked as inactive because it has not had recent activity. It will be closed in 15 days if no further activity occurs. Thank you for your contributions. |
Test cases for data validation function
#99